Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test(RHTAPWATCH-1068): Unit test for getNamespacesWithAccess #40

Merged
merged 1 commit into from
Aug 15, 2024
Merged

test(RHTAPWATCH-1068): Unit test for getNamespacesWithAccess #40

merged 1 commit into from
Aug 15, 2024

Conversation

hmariset
Copy link
Contributor

Add unit test for workspace-manager function
getNamespacesWithAccess

Jira-Url: https://issues.redhat.com/browse/RHTAPWATCH-1068

@hmariset hmariset marked this pull request as draft July 15, 2024 02:58
Omeramsc
Omeramsc requested changes Jul 16, 2024
View reviewed changes
cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
@hmariset hmariset force-pushed the RHTAPWATCH-1068 branch 2 times, most recently from c7a8bfb to 58f1b63 Compare July 18, 2024 05:04
Omeramsc
Omeramsc reviewed Jul 18, 2024
View reviewed changes
cmd/main_test.go Outdated Show resolved Hide resolved
Omeramsc
Omeramsc reviewed Jul 18, 2024
View reviewed changes
cmd/main_test.go Outdated Show resolved Hide resolved
Omeramsc
Omeramsc reviewed Jul 18, 2024
View reviewed changes
cmd/main_test.go Outdated Show resolved Hide resolved
@yftacherzog
Copy link
Member

/retest

@hmariset hmariset marked this pull request as ready for review July 24, 2024 03:46
@hmariset hmariset force-pushed the RHTAPWATCH-1068 branch 2 times, most recently from a7863da to e82c94a Compare July 25, 2024 00:54
Omeramsc
Omeramsc requested changes Jul 25, 2024
View reviewed changes
Copy link
Collaborator

@Omeramsc Omeramsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good progress! I left some comments

cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
@hmariset hmariset force-pushed the RHTAPWATCH-1068 branch 2 times, most recently from bb55c11 to 9ef6e76 Compare July 26, 2024 14:43
@Omeramsc Omeramsc mentioned this pull request Jul 28, 2024
Merged
Omeramsc
Omeramsc requested changes Jul 28, 2024
View reviewed changes
Copy link
Collaborator

@Omeramsc Omeramsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the line break on the testcase's name is breaking the code. PTAL

cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
@hmariset hmariset force-pushed the RHTAPWATCH-1068 branch 3 times, most recently from 955ad8f to ee8e9d4 Compare July 30, 2024 14:57
Omeramsc
Omeramsc requested changes Jul 31, 2024
View reviewed changes
cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
Omeramsc
Omeramsc requested changes Aug 1, 2024
View reviewed changes
cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
Omeramsc
Omeramsc reviewed Aug 4, 2024
View reviewed changes
cmd/main_test.go Outdated Show resolved Hide resolved
Omeramsc
Omeramsc requested changes Aug 6, 2024
View reviewed changes
Copy link
Collaborator

@Omeramsc Omeramsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resource managment between tests require fixing.

Also if my comments are not about typos/basic syntax, please comment on them with how you fixed them before resolving them (or wait for my resolution).

cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
@hmariset hmariset force-pushed the RHTAPWATCH-1068 branch 2 times, most recently from cfb38a2 to 32c5f1c Compare August 6, 2024 20:43
Omeramsc
Omeramsc requested changes Aug 11, 2024
View reviewed changes
Copy link
Collaborator

@Omeramsc Omeramsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good work! I left some comments to make the code a bit more reliable and concise. I believe we will be able to merge it afterwards.

cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
cmd/main_test.go Outdated Show resolved Hide resolved
@hmariset hmariset force-pushed the RHTAPWATCH-1068 branch 3 times, most recently from 244dced to a298992 Compare August 13, 2024 14:49
@hmariset
test(RHTAPWATCH-1068): Unit test for getNamespacesWithAccess
ed4cd38 
Add unit test for workspace-manager function
getNamespacesWithAccess

Jira-Url: https://issues.redhat.com/browse/RHTAPWATCH-1068
Signed-off-by: Homaja Marisetty <[email protected]>
Omeramsc
Omeramsc approved these changes Aug 14, 2024
View reviewed changes
Copy link
Collaborator

@Omeramsc Omeramsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Omeramsc Omeramsc merged commit 5f62bf1 into konflux-ci:main Aug 15, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avi-biton avi-biton avi-biton left review comments

@Omeramsc Omeramsc Omeramsc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hmariset @yftacherzog @Omeramsc @avi-biton